Skip to content

Conversation

@XingY
Copy link
Contributor

@XingY XingY commented Aug 14, 2025

Rationale

This PR is the follow up work for #6894 that went to 25.7, that made attachment columns rejecting string type values on import. This PR handles attachment value validation for insert/update api.

Related Pull Requests

Changes

Tasks 📍

if (isAttachmentProperty(name))
{
if (null != file.getFilename())
if (value instanceof AttachmentFile file)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Put more simply:

if (value instanceof AttachmentFile file)
{
    if (null != file.getFilename())
    {
        rowStripped.put(name, file.getFilename());
        attachments.put(name, value);
    }
}
else if (value instanceof String strVal && !StringUtils.isEmpty(strVal))
{
    // Issue 53498: string value for an attachment field is not allowed
    throw new ValidationException("Can't upload '" + strVal + "' to field " + name + " with type Attachment.");
}
else
    rowStripped.put(name, value); // if blank, remove the attachment

attachments.put(name, value);
}
}
else if (value instanceof String strVal)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this leave it open to being another type of value besides a File or String (number, date, boolean, etc.)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed made to also check for non string types (excluding attachment type). Similar change is also made for FileLink fields. Tests added for both attachment and filelink.

@XingY XingY requested a review from labkey-nicka August 17, 2025 23:54
@XingY XingY merged commit a259e08 into develop Aug 18, 2025
14 of 15 checks passed
@XingY XingY deleted the fb_attachmentAPI branch August 18, 2025 21:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants